Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add set_bit to buint #44

Merged
merged 2 commits into from
Sep 18, 2024
Merged

add set_bit to buint #44

merged 2 commits into from
Sep 18, 2024

Conversation

krakow10
Copy link
Contributor

Hi, is there room for more digit manipulation without needing to copy the whole value? This is one thing I would like to have.

@isaacholt100
Copy link
Owner

Hi @krakow10, thanks for this pull request. I'm happy to add more digit manipulation, what do you specifically want? I will merge this PR, but first could you add documentation for set_bit by creating a set_bit macro in doc/mod.rs, use the bit macro as an example, then add #[doc = doc::set_bit!(U 256)] to the definition instead of #[doc = doc::bit!(U 256)]. Thanks.

@krakow10
Copy link
Contributor Author

krakow10 commented Sep 5, 2024

I wrote a documentation like you requested. Can you think of a better way to set the bit? For example, do you think $Digit::from(value) is problematic and should be done differently?

I'm happy to add more digit manipulation, what do you specifically want?

I thought I would come up with more digit manipulation needs, but I didn't run into anything else that inspired a useful method. I will propose more changes later if I think of something useful, thanks.

P.S. I couldn't test this because of the unstable features

@isaacholt100
Copy link
Owner

Sorry for the very slow response time @krakow10, I've just come back from a holiday. Thanks for adding the documentation, I will merge this now. Sorry about the compilation error when trying to test - annoyingly, the nightly compiler seems to have recently dropped const trait support, which is what was causing that issue. I'm going to make those traits non-const for now. I'll publish 0.12.0 with both these changes either today or tomorrow, hopefully.

@isaacholt100 isaacholt100 changed the base branch from master to latest September 18, 2024 17:59
@isaacholt100 isaacholt100 merged commit 8e207de into isaacholt100:latest Sep 18, 2024
2 checks passed
@isaacholt100
Copy link
Owner

I think there may be a slight error in your code actually, as applying & (1 << shift) will zero out other bits - I've modified it slightly so that it's fully correct, by doing

if value {
    *digit |= (1 << shift);
} else {
    *digit &= !(1 << shift);
}

@krakow10
Copy link
Contributor Author

I think it was supposed to be a reverse mask, but I forgot !. Like this:

*digit = *digit & !(1 << shift) | ($Digit::from(value) << shift)

Saves a branch that way.

@isaacholt100
Copy link
Owner

Ah yeah I see, I think a branch is unavoidable though as $Digit::from(value) uses one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants